Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Correctly handle PKCS#11 tokens for system keystore #33460

Merged
merged 4 commits into from
Sep 10, 2018
Merged

Correctly handle PKCS#11 tokens for system keystore #33460

merged 4 commits into from
Sep 10, 2018

Conversation

jkakavas
Copy link
Member

@jkakavas jkakavas commented Sep 6, 2018 •
edited
Loading

  • Ensure we handle the NONE keyword for system keystores correctly

    As defined in the PKCS#11 reference guide
    https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
    PKCS#11 tokens can be used as the JSSE keystore and truststore and
    the way to indicate this is to set javax.net.ssl.keyStore and
    javax.net.ssl.trustStore to NONE (case sensitive).
    This commits ensures that we honor this convention and do not
    attempt to load the keystore or truststore if the system property is
    set to NONE.

  • Ensure we handle password protected PKCS#11 tokens used as
    system truststores correctly.

    When a PKCS#11 token is used as the system truststore, we need to
    pass a password when loading it, even if only for reading
    certificate entries. This commit ensures that if
    javax.net.ssl.trustStoreType is set to PKCS#11 (as it would
    when a PKCS#11 token is in use) the password specified in
    javax.net.ssl.trustStorePassword is passed when attempting to
    load the truststore.

Relates #33459

@jkakavas
Correctly handle NONE keyword for system keystore
a4152f7 
As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

Relates elastic#33459
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

@jkakavas jkakavas mentioned this pull request Sep 6, 2018
Closed
jaymode
jaymode approved these changes Sep 6, 2018
View reviewed changes
Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkakavas
Handle password protected system truststore
3de4aef 
When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.
@jkakavas jkakavas changed the title Correctly handle NONE keyword for system keystore Correctly handle PKCS#11 tokens for system keystore Sep 6, 2018
@jkakavas
Copy link
Member Author

jkakavas commented Sep 6, 2018

@jaymode I widened the scope of this PR slightly as the changes fitted well together, could you take another look please ?

jaymode
jaymode approved these changes Sep 6, 2018
View reviewed changes
Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jkakavas
Copy link
Member Author

jkakavas commented Sep 6, 2018

CI failed with

FAILURE 0.03s J1 | DateTimeUnitTests.testConversion <<< FAILURES!
   > Throwable #1: java.lang.AssertionError: 
   > Expected: is <783512977353L>
   >      but: was <783509377353L>
   > 	at __randomizedtesting.SeedInfo.seed([A473697370F6B495:A27BEC495D39EB01]:0)
   > 	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   > 	at org.elasticsearch.common.rounding.DateTimeUnitTests.testConversion(DateTimeUnitTests.java:79)
   > 	at java.lang.Thread.run(Thread.java:748)
Completed [958/1128] on J1 in 0.04s, 2 tests, 1 failure <<< FAILURES!

This has never failed in the past , this looks unrelated to this PR and possibly related to #32641 (ping @spinscale for visibility), although it doesn't reproduce locally.

./gradlew :server:test -Dtests.seed=A473697370F6B495 -Dtests.class=org.elasticsearch.common.rounding.DateTimeUnitTests -Dtests.method="testConversion" -Dtests.security.manager=true -Dtests.locale=en-NZ -Dtests.timezone=Pacific/Honolulu -Dcompiler.java=10 -Druntime.java=8

@jkakavas
Copy link
Member Author

jkakavas commented Sep 6, 2018

Jenkins test this please

@jkakavas jkakavas merged commit 77aeeda into elastic:master Sep 10, 2018
jkakavas added a commit that referenced this pull request Sep 10, 2018
@jkakavas
Correctly handle PKCS#11 tokens for system keystore (#33460)
3bc7e1c 
* Correctly handle NONE keyword for system keystore

As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

* Handle password protected system truststore

When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.

Relates #33459
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Sep 11, 2018
@jasontedor
Merge branch 'master' into upgrade-remote-cluster-settings
f82e74d 
* master:
  Add full cluster restart base class (elastic#33577)
  Validate list values for settings (elastic#33503)
  Copy and validatie soft-deletes setting on resize (elastic#33517)
  Test: Fix package name
  SQL: Fix result column names for arithmetic functions (elastic#33500)
  Upgrade to latest Lucene snapshot (elastic#33505)
  Enable not wiping cluster settings after REST test (elastic#33575)
  MINOR: Remove Dead Code in SearchScript (elastic#33569)
  [Test] Remove duplicate method in TestShardRouting (elastic#32815)
  mute test on windows
  Update beats template to include apm-server metrics (elastic#33286)
  Fix typos (elastic#33499)
  [CCR] Delay auto follow license check (elastic#33557)
  [CCR] Add create_follow_index privilege (elastic#33559)
  Strengthen FilterRoutingTests (elastic#33149)
  Correctly handle PKCS#11 tokens for system keystore (elastic#33460)
  Remove some duplicate request conversion methods. (elastic#33538)
@jkakavas jkakavas deleted the pkcs11-fix branch September 14, 2018 06:44
@jkakavas jkakavas added v6.4.3 and removed v6.4.2 labels Oct 5, 2018
jkakavas added a commit that referenced this pull request Oct 5, 2018
@jkakavas
Correctly handle PKCS#11 tokens for system keystore (#33460)
9a088c9 
* Correctly handle NONE keyword for system keystore

As defined in the PKCS#11 reference guide
https://docs.oracle.com/javase/8/docs/technotes/guides/security/p11guide.html
PKCS#11 tokens can be used as the JSSE keystore and truststore and
the way to indicate this is to set `javax.net.ssl.keyStore` and
`javax.net.ssl.trustStore` to `NONE` (case sensitive).

This commits ensures that we honor this convention and do not
attempt to load the keystore or truststore if the system property is
set to NONE.

* Handle password protected system truststore

When a PKCS#11 token is used as the system truststore, we need to
pass a password when loading it, even if only for reading
certificate entries. This commit ensures that if
`javax.net.ssl.trustStoreType` is set to `PKCS#11` (as it would
when a PKCS#11 token is in use) the password specified in
`javax.net.ssl.trustStorePassword` is passed when attempting to
load the truststore.

Relates #33459
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaymode jaymode jaymode approved these changes

Assignees
No one assigned
Labels
>bug :Security/TLS SSL/TLS, Certificates v6.4.3 v7.0.0-beta1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jkakavas @elasticmachine @jaymode @jimczi